Skip to content

[clang Dependency Scanning] Out-of-Date Scanning File System Cache Entry Reporting C-APIs #10927

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 11 commits into from
Jul 11, 2025
Merged

[clang Dependency Scanning] Out-of-Date Scanning File System Cache Entry Reporting C-APIs #10927

merged 11 commits into from
Jul 11, 2025

Conversation

qiongsiwu
Copy link

@qiongsiwu qiongsiwu commented Jun 30, 2025 •
edited
Loading

This PR implements the C-APIs to report a scanning file system cache's out-of-date entries. The C-APIs contains a function to return a set of file system cache out-of-date entries, functions to facilitate looping through all the entries, and reporting the relevant information from the entries.

The APIs are based on llvm#144105.

rdar://152247357

@qiongsiwu
Copy link
Author

Note to reviewers: I don't see a good way to cleanly test these APIs through c-index tests. Ideally, I'd like to create an underlying service whose file system cache I can manipulate directly to test these APIs directly (something similar to https://github.com/llvm/llvm-project/pull/144105/files#diff-25ec4b41a8aed83f312f2bb7b409f379f38dd9d1597d6e4e45e17681260dfcc2R205).

Do we think it is reasonable to create unit tests for these C-APIs? If so, I will try making some unit tests.

owenv
owenv reviewed Jun 30, 2025
View reviewed changes
@qiongsiwu qiongsiwu requested a review from owenv June 30, 2025 21:38
@qiongsiwu qiongsiwu self-assigned this Jul 2, 2025
@qiongsiwu qiongsiwu marked this pull request as ready for review July 2, 2025 18:15
@qiongsiwu
Copy link
Author

@swift-ci please test llvm.

@qiongsiwu
Copy link
Author

@swift-ci please test llvm.

@qiongsiwu
Copy link
Author

qiongsiwu commented Jul 2, 2025 •
edited
Loading

The two failing tests do not relate to this PR. rdar://154865941 is tracking them.

@qiongsiwu
Copy link
Author

Gentle ping for review. Thanks!

jansvoboda11
jansvoboda11 reviewed Jul 7, 2025
View reviewed changes
jansvoboda11
jansvoboda11 reviewed Jul 7, 2025
View reviewed changes
jansvoboda11
jansvoboda11 reviewed Jul 7, 2025
View reviewed changes
@jansvoboda11
Copy link

Adding the unit test is a nice bonus, but I'd be in favor in keeping the old c-index-test test around. I think it's nice that c-index-test is a nice one-stop-shop for anything libclang.

@qiongsiwu
Copy link
Author

qiongsiwu commented Jul 7, 2025 •
edited
Loading

Adding the unit test is a nice bonus, but I'd be in favor in keeping the old c-index-test test around. I think it's nice that c-index-test is a nice one-stop-shop for anything libclang.

This is a good point. The difficulty is that it is not easy to create out-date-entries exactly as we desired to make sure we have good coverage of all the APIs. I can recover the removed c-index-test that at least runs through the APIs obtaining the out-of-date entry set and disposing it. The set will contain no entry. Does that sound reasonable?

@qiongsiwu qiongsiwu requested a review from jansvoboda11 July 7, 2025 19:19
@qiongsiwu
Copy link
Author

@swift-ci please test llvm.

@qiongsiwu
Copy link
Author

@swift-ci please test llvm.

@qiongsiwu qiongsiwu requested a review from jansvoboda11 July 8, 2025 16:47
@qiongsiwu
Copy link
Author

@swift-ci please test llvm.

jansvoboda11
jansvoboda11 approved these changes Jul 10, 2025
View reviewed changes
Copy link

@jansvoboda11 jansvoboda11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thank you! LGTM sans the small nits.

clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think changes here should also go upstream, right? (It's fine to do as a follow-up to reduce churn and waiting.)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup I agree! I will make an upstream PR afterwards.

@qiongsiwu
Copy link
Author

@swift-ci please test llvm.

@qiongsiwu
Copy link
Author

qiongsiwu commented Jul 11, 2025 •
edited
Loading

I took a look at the failing tests and they are not related to this PR. I am landing the PR. Thanks so much for the detailed review @jansvoboda11 !

@qiongsiwu qiongsiwu merged commit 6fbcea6 into swiftlang:next Jul 11, 2025
0 of 2 checks passed
@qiongsiwu qiongsiwu mentioned this pull request Jul 11, 2025
Merged
qiongsiwu added a commit that referenced this pull request Jul 12, 2025
@qiongsiwu
Adding missing visibility attributes to out-of-date entry diagnostics…
d7f7e88 
… C-APIs. (#10992)

#10927 added a set of C-APIs to report out-of-date scanning file system cache entires. The C-APIs missed their visibility attributes. This PR adds `CINDEX_LINKAGE` to all APIs added by #10927. 

rdar://152247357
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jansvoboda11 jansvoboda11 jansvoboda11 approved these changes

@Bigcheese Bigcheese Awaiting requested review from Bigcheese

@cachemeifyoucan cachemeifyoucan Awaiting requested review from cachemeifyoucan

@owenv owenv Awaiting requested review from owenv

Assignees

@qiongsiwu qiongsiwu

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@qiongsiwu @jansvoboda11 @owenv